Skip to content

Allow to queries IP prefixes#253

Open
romain-fontugne wants to merge 11 commits into
secynic:devfrom
romain-fontugne:dev
Open

Allow to queries IP prefixes#253
romain-fontugne wants to merge 11 commits into
secynic:devfrom
romain-fontugne:dev

Conversation

@romain-fontugne

Copy link
Copy Markdown

This allows to query an IP prefix, for example 8.8.8.0/24 or 8.0.0.0/9
The is_defined check is done with the first IP in the range.

fixes #250

@coveralls

coveralls commented Mar 2, 2020

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.003%) to 99.648% when pulling 72bb764 on romain-fontugne:dev into 11f7632 on secynic:dev.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.004%) to 99.217% when pulling 8391fcb on romain-fontugne:dev into 282fb01 on secynic:dev.

@secynic
secynic self-requested a review March 4, 2020 22:50

@secynic secynic left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix 2.7 build

Comment thread ipwhois/tests/test_net.py Outdated
self.assertRaises(IPDefinedError, Net, IPv6Address('fe80::'))
self.assertRaises(IPDefinedError, Net, '192.168.0.0/16')
self.assertRaises(IPDefinedError, Net, 'fe80::/10')
self.assertRaises(IPDefinedError, Net, IPv4Network('192.168.0.0/16'))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not building on Python 2.7. I think it has to do with your addition of unicode() which seems unnecessary.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed that, the problem was that the test used ipaddr.IPv4Network but now the net.py code check for ipaddress.IPv4Network. ipaddr has been superseded by ipaddress for both python3 and python2. It might be better to replace all the instances of ipaddr to ipaddress across all the files?

@secynic

secynic commented Sep 1, 2020

Copy link
Copy Markdown
Owner

@romain-fontugne Still not building for 2.7. ipaddress is not built in on Python 2.7

@secynic

secynic commented Sep 2, 2020

Copy link
Copy Markdown
Owner

@romain-fontugne Actually, after I have reviewed this, I don't see the point in this if it only uses the first address in a range. Can you explain why this is needed?

@romain-fontugne

Copy link
Copy Markdown
Author

@secynic sorry, I haven't take the time to look at that. Only the is_defined check is done with the first IP in the range. The query is done with the prefix. Prefix 1.2.0.0/16 and 1.2.0.0/24 have the same first IP but could have different whois info.

@alistairking

Copy link
Copy Markdown

I could really make use of this feature too.
What still needs to happen before it can be merged?

@romain-fontugne

Copy link
Copy Markdown
Author

Hey Alistair,
The code works for python3 but it seems to break some python2.7 tests. I never took the time to debug this... so I am not sure what's the problem.

@secynic

secynic commented Oct 13, 2024

Copy link
Copy Markdown
Owner

Probably removing python 2 support. Will revisit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants